Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add baking script for archival bakes #4502

Merged
merged 27 commits into from
Feb 11, 2025
Merged

feat: add baking script for archival bakes #4502

merged 27 commits into from
Feb 11, 2025

Conversation

marcelgerber
Copy link
Member

@marcelgerber marcelgerber commented Jan 29, 2025 •
edited
Loading

This implements milestone 1 of #4434: correctly referencing assets in an archival bake.

It:

  • content-hashes
    • JS & CSS assets
    • dods.json files
    • Data & metadata files
  • puts them into staticAssetMap and runtimeAssetMap, respectively
  • makes sure these are both propagated to the correct function calls

To run this locally, run the following:

yarn buildViteSite # builds JS & CSS into dist/ folder
VITE_PREVIEW=true npx tsx --tsconfig tsconfig.tsx.json ./baker/buildLocalArchivalBake.ts --latestDir # build the latest archival build into archive/ dir
# VITE_PREVIEW is needed for now to correctly reference JS assets
npx serve archive/ # serves on localhost:3000

You can then navigate to http://localhost:3000/latest/grapher/life-expectancy to view the page.

@owidbot
Copy link
Contributor

owidbot commented Jan 29, 2025 •
edited
Loading

Quick links (staging server):

Site Dev Site Preview Admin Wizard Docs

Login: ssh owid@staging-site-archival-baker

SVG tester:

Number of differences (default views): 0 ✅
Number of differences (all views): 0 ✅

Edited: 2025-02-11 11:16:52 UTC
Execution time: 1.19 seconds

@marcelgerber marcelgerber force-pushed the archival-baker branch 3 times, most recently from 4ffb030 to 8ce7d67 Compare February 4, 2025 17:34
@marcelgerber marcelgerber requested a review from rakyi February 4, 2025 17:34
@marcelgerber marcelgerber marked this pull request as ready for review February 4, 2025 17:35
@marcelgerber marcelgerber mentioned this pull request Jan 31, 2025
Open
28 tasks
rakyi
rakyi reviewed Feb 6, 2025
View reviewed changes
Copy link
Contributor

@rakyi rakyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Couple of notes:

  • Loading the whole file into memory before hashing might consume a lot of memory when we start doing full archival bakes. I guess it's fine to solve this (by switching to streams) once we hit the problem in practice, so this is more of an FYI to keep it in mind.
  • Since the asset map is never gonna change, we can save ourselves quite a bit of trouble by attaching it to window and reading it from there every time we need to check it, instead of passing it via props. At least for the runtime map, might not be true for the static. Or am I missing something?

serverUtils/hash.test.ts Outdated Show resolved Hide resolved
baker/buildLocalArchivalBake.ts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want errors from this script reported in Sentry. See e.g. baker/algolia/indexChartsToAlgolia.ts for an example on how to do it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unresolving, since I don't see the import of serverUtils/instrument.js at the top. You can copy what's in indexChartsToAlgolia.ts.

baker/buildLocalArchivalBake.ts Outdated Show resolved Hide resolved
@marcelgerber
wip: add (very basic) bake script for archival
d97fecd
@marcelgerber
feat(bake): allow passing in an assetMap
c498e9f
@marcelgerber
enhance: pass assetMap all the way down
a536689
@marcelgerber
enhance: actually hash & copy over assets
e403c34
@marcelgerber
enhance: bake dods & variable files
c3162c9
@marcelgerber
enhance: resolve runtime assets, too, and change to absolute paths
9eaebd9
@marcelgerber
fix: use correct assets in preload directives
67697df
@marcelgerber
enhance: better asset path construction
f24dd90
@marcelgerber
enhance: use base36 hashes
00aca45
@marcelgerber
enhance: create a manifest alongside archived grapher page
ac540ce
@marcelgerber
enhance: put grapher page into date-stamped dir
f105b7e
@marcelgerber
refactor: rename viteAssetMap -> staticAssetMap
f4e5a9d
@marcelgerber
enhance: copy over static files from public/ folder
346bc58
@marcelgerber
refactor: rename AssetMapEntry -> AssetMap
f2444b3
@marcelgerber
refactor: pull out hashing
b677f39
@marcelgerber
enhance: build some safety around assetMap reads
7cc3235
@marcelgerber
enhance: allow copying to latest dir
d79497c
@marcelgerber
enhance: make source maps work
0568a34
@marcelgerber
feat: directly hash a stream
1a031a7
@marcelgerber
enhance: properly content-hash owid.mjs file
d163d7d
@marcelgerber
Copy link
Member Author

  • Since the asset map is never gonna change, we can save ourselves quite a bit of trouble by attaching it to window and reading it from there every time we need to check it, instead of passing it via props. At least for the runtime map, might not be true for the static. Or am I missing something?

Yep, you're absolutely right. I considered doing this, and then didn't end up doing it, but for no good reason really.

I've now changed things around and am putting these under window._OWID_RUNTIME_ASSET_MAP.

rakyi
rakyi reviewed Feb 11, 2025
View reviewed changes
Copy link
Contributor

@rakyi rakyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! LGTM except the missing instrumentation import.

@marcelgerber marcelgerber merged commit 6f6d734 into master Feb 11, 2025
15 of 18 checks passed
@marcelgerber marcelgerber deleted the archival-baker branch February 11, 2025 15:59
@sentry-io Sentry.io
Copy link

sentry-io bot commented Feb 11, 2025 •
edited
Loading

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ Error: Data page with slug "antibiotic-use-in-livestock-in-europe" is using "Antibiotics" as its primary tag, which we are unable to resolve to a tag in the grapher DB getPrimaryTopic(DatapageHelpers.ts) View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rakyi rakyi rakyi left review comments

Assignees

@marcelgerber marcelgerber

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@marcelgerber @owidbot @rakyi